Fix DateTime: overload TAILCALL, method name resolution, and spurious warnings (99.7% tests pass)#344
Merged
Merged
Conversation
fglock
added a commit
that referenced
this pull request
Mar 20, 2026
Brings in: - Simplified overload.pm with no strict refs (fixes most overload issues) - Phase 13 overload stringification fix - Updated design docs Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
b18c70e to
d9a667c
Compare
Phase 14 DateTime fixes: 1. _ymd2rd day overflow/underflow handling: - DateTime uses day=0 for 'last day of previous month' - DateTime uses day > last_day for overflow to next month - Changed from clamping to LocalDate.plusDays() approach 2. Overload TAILCALL trampoline: - goto $coderef in overload handlers was not being executed - RuntimeControlFlowList (TAILCALL marker) has no list elements - getFirst() was returning 0/undef instead of actual result - Added trampoline loop in OverloadContext.tryOverload() Test results: 3280/3302 subtests passed (99.3%) - t/06add.t: 2 -> 0 failures (fixed) - t/07compare.t: 1 -> 0 failures (fixed) - t/10subtract.t: 4 -> 0 failures (fixed) - t/11duration.t: 4 -> 0 failures (fixed) - t/27delta.t: 4 -> 0 failures (fixed) - t/38local-subtract.t: 7 -> 2 failures (5 fewer) - Total: 32 -> 22 failures (10 fewer) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The LEAP_SECONDS table in DateTime.java had incorrect Rata Die (RD) day values, causing all leap second tests to fail. The values were off by approximately 8800 days (~24 years). Fixed by updating the table with correct values from the official DateTime leap_seconds.h from CPAN: - 1972-07-01: was 728896, now 720075 - All 27 leap second entries corrected Test results: - t/19leap-second.t: 204/204 pass (was 12 failures) - t/32leap-second2.t: 57/57 pass (was 7 failures) - t/38local-subtract.t: 127/127 pass (was 2 failures) - DateTime overall: 3481/3482 subtests pass (99.97%) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Categorized remaining DateTime test issues: - Parse errors (14): Cosmetic only, tests pass - Missing CPAN deps (3): Optional, can install via jcpan - jcpan share/ support (1): Enhancement needed for locale data - PerlOnJava bugs (2): overload.pm symbol resolution, Dist::CheckConflicts - By design (1): namespace::autoclean Next steps prioritized: 1. overload.pm symbol resolution (Medium) 2. jcpan share/ directory support (Medium) DateTime is now 99.97% passing (3481/3482 subtests). Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Analysis shows namespace::autoclean can be properly implemented:
- Sub::Util::subname() correctly identifies imported subs
- undef *{Package::sub} works to remove subs from symbol table
- B::Hooks::EndOfScope already works via defer mechanism
The original stub comment was incorrect - cleanup doesn't break
Try::Tiny usage because cleanup happens AFTER compilation.
Updated Phase 16 priorities:
1. namespace::autoclean implementation (HIGH)
2. overload.pm symbol resolution (MEDIUM)
3. jcpan share/ directory support (MEDIUM)
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Found the actual root cause of the overload.pm error: - PerlOnJava's overload.pm is missing `no strict 'refs';` at package level - Perl 5.42's overload.pm has this on line 4 - The mycan() function needs it to create glob refs from strings This is a one-line fix in src/main/perl/lib/overload.pm. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Documented how File::ShareDir works: - CPAN dists have share/ directories with data files - Install to auto/share/dist/Dist-Name/ in lib path - File::ShareDir::dist_dir() searches @inc for this path Implementation steps for jcpan: 1. Detect share/ directory after extracting dist 2. Copy contents to ~/.perlonjava/auto/share/dist/Dist-Name/ 3. File::ShareDir should already find it if path is in @inc Example: DateTime-Locale has 1070 locale files (fr.pl, de.pl, etc.) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The overload.pm module requires 'no strict refs' at package level
because mycan() uses symbolic references like \*{$fqmeth}. Our
previous version was missing this pragma, causing failures in
modules that depend on overload introspection (like DateTime with
Specio type constraints).
Changes:
- Import overload.pm and overloading.pm from perl5 source
- Add these modules to import config for future syncs
- Both modules now have proper 'no strict refs' declarations
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
When code blocks are passed to List::Util functions (any, all, first, reduce, reductions, pairmap, pairgrep, pairfirst), the blocks should have access to the outer subroutine's @_ via $_[0], $_[1] etc. Previously, $_[0] inside these blocks was either empty or contained the List::Util function's own arguments instead of the caller's @_. The fix adds: 1. A thread-local stack in RuntimeCode that tracks @_ for each subroutine call frame 2. pushArgs/popArgs calls in RuntimeCode.apply() to maintain the stack 3. getCallerArgs() method to retrieve the caller's @_ (one level up) 4. Updates to all List::Util block-taking functions to pass the caller's @_ to the code blocks This fixes Specio union type constraints which use List::Util::any with closures that reference outer @_. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…toquoting
Phase 11 completion:
1. namespace::autoclean stub (src/main/perl/lib/namespace/autoclean.pm)
- Provides interface but skips cleanup entirely
- Allows imported functions (Try::Tiny try/catch) to remain available
- Fixes Undefined subroutine DateTime::TimeZone::catch error
2. Parser fix for keyword autoquoting (ListParser.java)
- Extended AUTOQUOTABLE_KEYWORDS set to include: if, unless, while,
until, for, foreach (in addition to and, or, xor, when)
- Keywords followed by => are now treated as bareword hash keys
- Fixes Expected token } but got until parser error
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Fix refaddr to return identity hash of underlying object, not wrapper This fixes Specio enum validation and DateTime truncate/today - Add POSIX math functions: floor, ceil, fmod, fabs, pow, trig, etc. DateTime uses POSIX::floor and POSIX::fmod - Update cpan_client.md with Phase 12 completion details DateTime now uses Java XS implementation ($DateTime::IsPurePerl = 0) with 98.5% of DateTime tests passing. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Added comprehensive list of remaining DateTime test failures (45/3292): - Overload stringification StackOverflowError (HIGH PRIORITY) - Leap second handling issues (19 failures) - End-of-month arithmetic bugs (21 failures) - Missing test dependencies - DateTime::Locale data file installation - IPC::Open3 read-only modification bug - Dist::CheckConflicts method resolution - Encode::PERLQQ undefined - Number::Overloaded integration Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- t/20infinite.t: All 104 tests now pass - t/31formatter.t: All 11 tests now pass - DateTime test results improved: 3260/3302 (98.7%), down from 45 to 42 failures Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Document the three fixes for DateTime test suite: - _ymd2rd day overflow/underflow handling - Leap second table correction - TAILCALL trampoline in overload context Test results improved from 47 failures to 7 failures (99.8% pass rate). Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
When overload is defined with a method name string (not code ref): use overload '""' => '_stringify'; # method name Perl's overload.pm stores: - CODE slot: \&overload::nil (a no-op function) - SCALAR slot: the method name string PerlOnJava was just executing the CODE slot and getting undef. Now we detect overload::nil and resolve the actual method from the SCALAR slot. This fixes Specio type stringification which was breaking DateTime's validation system: - Before: 96.3% pass rate (1987/2064) - After: 99.7% pass rate (3513/3522) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- NumberParser: Check if 'numeric' warnings are enabled before emitting "Argument isn't numeric" warning. Fixes spurious warnings during DateTime tests where Test::Builder uses `no warnings 'numeric'` but the warning was still being generated. - Operator/RuntimeSubstrLvalue: Check if 'substr' warnings are enabled before emitting "substr outside of string" warning. Fixes spurious warnings during Test::More skip() operations. - Update design doc with detailed breakdown of remaining DateTime test failures (4 TODO tests, 2 missing warning tests, 3 dependency tests). Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Add checkUninitialized() to CompareOperators for greaterThan - Import Term::ANSIColor module via sync.pl config - Warnings now generated when comparing undef with numeric operators DateTime t/29overload.t: 30/32 pass (was 28/32) Missing: location info in runtime warnings (pre-existing limitation) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Uninitialized value warnings now working for numeric comparisons - Term::ANSIColor imported - 7 remaining failures (4 TODO, 2 location, 1 format) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Required by Parse::CPAN::Meta for reading META.json files. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Change prototype from "$" to "$$" to accept both code ref and prototype string - Return the code reference instead of undef, matching Perl behavior - Handle undef prototype argument by setting prototype to null This fixes Test2::Mock loading which imports set_prototype from Scalar::Util. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Use invocant ($self) as the SOURCE package, not a positional argument - Discard the redundant 3rd argument like Perl's Exporter::Heavy does - Use caller($level) to determine target package (was already correct) This fixes Dist::CheckConflicts which uses export_to_level in its import(). The exported methods (conflicts, check_conflicts, etc.) now work correctly. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
PerlOnJava uses 32-bit integers and doesn't automatically convert large integers to scientific notation like Perl 5 does. This caused the BEGIN block that detects max integer size to never set $max_intsize, leaving it undef. Add a patch to set $max_intsize to 9 (safe for 32-bit) when the overflow detection loop doesn't find scientific notation. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
d9a667c to
c599fe1
Compare
- RuntimeSubstrLvalue.set(): die instead of warn for out-of-bounds offset
- Operator.java: remove redundant Warnings import
- lvalue_substr.t: fix test 2 to expect die, not warn (matches Perl 5)
- Add warnings.t: test warn/die behavior for substr and {__WARN__}
- Add WARNINGS_RUNTIME_FIX.md: document warnings::enabled() issue
Perl 5 behavior:
- substr($str, 10, 5) = 'x' -> dies with 'substr outside of string'
- $x = substr($str, 10, 5) -> warns and returns undef
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
4aafa87 to
d493a1e
Compare
- Add dedicated handleSubstrOperator() in EmitOperator.java that checks
compile-time warning state before generating code
- Add substr case to EmitOperatorNode.java switch statement
- Add "substr" to default enabled warnings in initializeEnabledWarnings()
- Add SUBSTR_VAR_NO_WARN opcode for interpreter backend
- Revert numeric warning check in NumberParser to match master behavior
Root cause: use warnings set flags in ScopedSymbolTable at compile time,
but the check was using isWarningEnabled() which tried to read runtime
state. The fix checks symbolTable.isWarningCategoryEnabled() at compile
time and emits the appropriate method call.
Test: ./jperl -e 'use warnings; substr("a", 3);' now warns
./jperl -e 'no warnings "substr"; substr("a", 3);' is silent
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
d493a1e to
3dec35e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 14-16 DateTime fixes addressing multiple issues:
Phase 14: _ymd2rd day overflow/underflow handling
The Java XS _ymd2rd function was clamping day values to valid range instead of allowing overflow/underflow. DateTime relies on special day handling:
Fix: Changed from clamping to LocalDate.plusDays() which correctly handles all cases.
Phase 14: Overload TAILCALL trampoline
DateTime's _string_compare_overload method uses goto $meth to delegate to _compare_overload. When goto $coderef is used in an overload handler, it creates a TAILCALL marker. However, OverloadContext.tryOverload() was calling RuntimeCode.apply().getFirst() without handling TAILCALL markers.
Fix: Added trampoline loop in OverloadContext.tryOverload() to execute TAILCALL markers.
Phase 15: Overload method name resolution
Perl's overload pragma allows method name strings (e.g.,
'""' => '_stringify'). When a method name string is used, Perl's overload.pm stores the actual method name in the SCALAR slot and\&overload::nilin the CODE slot. PerlOnJava was missing this logic.Fix: Modified OverloadContext.tryOverload() to detect
overload::niland resolve the actual method name from the SCALAR slot.Phase 16: Compile-time warning checks for substr
Substr warnings were not being emitted even with
use warningsbecause:initializeEnabledWarnings()did not include "substr" in the default warning setFix:
initializeEnabledWarnings()handleSubstrOperator()in EmitOperator.java that checks compile-time warning stateOperator.substr()(with warning) orOperator.substrNoWarn()(no warning) based on compile-timeno warnings "substr"stateNote: Did NOT use
enableWarning("all")because that enables warnings like "uninitialized" which cause extra fetches from tied variables, breaking tests like gmagic.t.Test:
Test Results
DateTime test suite: 3513/3522 subtests passed (99.7%), 9 failures
Remaining Failures (non-critical)
These failures are due to:
Files Changed
Phase 14
src/main/java/org/perlonjava/runtime/perlmodule/DateTime.java- Fixed_ymd2rd, corrected leap second tablesrc/main/java/org/perlonjava/runtime/runtimetypes/OverloadContext.java- Added TAILCALL trampolinePhase 15
src/main/java/org/perlonjava/runtime/runtimetypes/OverloadContext.java- AddedresolveOverloadMethodName()helper, detectoverload::nilPhase 16
src/main/java/org/perlonjava/backend/jvm/EmitOperatorNode.java- Route substr to dedicated handlersrc/main/java/org/perlonjava/backend/jvm/EmitOperator.java- AddedhandleSubstrOperator()with compile-time warning checksrc/main/java/org/perlonjava/runtime/runtimetypes/WarningFlags.java- Added "substr" to default enabled warningssrc/main/java/org/perlonjava/runtime/operators/Operator.java- AddedsubstrNoWarn()methodsrc/main/java/org/perlonjava/backend/bytecode/Opcodes.java- Added SUBSTR_VAR_NO_WARN opcodesrc/main/java/org/perlonjava/backend/bytecode/CompileOperator.java- Emit correct opcode based on warning statesrc/main/java/org/perlonjava/backend/bytecode/BytecodeInterpreter.java- Handle new opcodesrc/main/java/org/perlonjava/backend/bytecode/OpcodeHandlerExtended.java- Execute new opcodesrc/main/java/org/perlonjava/backend/bytecode/Disassemble.java- Disassemble new opcodeGenerated with Devin